Skip to content
This repository has been archived by the owner on Dec 18, 2017. It is now read-only.

Fix GetRelativePath with different volumes #2685

Merged
merged 1 commit into from
Sep 11, 2015
Merged

Conversation

BrennanConroy
Copy link
Member

@@ -86,6 +86,12 @@ public static string GetRelativePath(string path1, string path2, char separator)
if (Microsoft.Dnx.Runtime.RuntimeEnvironmentHelper.IsWindows)
{
compare = StringComparison.OrdinalIgnoreCase;
// check if paths are on the same volume
if (string.Equals(path1.Substring(1, 1), ":") && !string.Equals(path1.Substring(0, 2), path2.Substring(0, 2), compare))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible index out of range exception

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that the best way to get the drive letter?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path.GetPathRoot

@BrennanConroy
Copy link
Member Author

Updated

new object[] { "../folder1/file", "C:\\folder\\file", "C:\\folder1\\file" },
new object[] { "H:\\folder\\file", "C:\\folder\\file", "H:\\folder\\file" },
new object[] { "folder/file", "C:\\", "C:\\folder\\file" },
new object[] { "..\\", "C:\\folder\\file", "C:\\" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the result here correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@davidfowl
Copy link
Member

:shipit: Once travis passes

@troydai
Copy link
Contributor

troydai commented Sep 11, 2015

:shipit:

@BrennanConroy BrennanConroy merged commit 8ec9f73 into dev Sep 11, 2015
@BrennanConroy BrennanConroy deleted the brecon/volume_relative branch September 11, 2015 23:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants